Fix: Handle SSE ping messages in WebFluxSseClientTransport#272
Fix: Handle SSE ping messages in WebFluxSseClientTransport#272goecho wants to merge 2 commits intomodelcontextprotocol:mainfrom
Conversation
- Add check for SSE ping messages from fastMCP server - Log debug message when ping received instead of throwing exception - Complete the stream on ping events - Matches behavior of Python SDK which treats pings as keep-alive
|
Hi @goecho , can you please elaborate on this. I'm not able to find |
|
Hi @tzolov this behavior is not defined in the MCP protocol itself—it's specific to the Python implementation using the fastmcp library. The behavior comes from the keep-alive mechanism in its dependency, the sse-starlette library. You can refer to the code here: https://github.com/sysid/sse-starlette/blob/main/sse_starlette/sse.py#L214 |
|
Hi @YunKuiLu ,I don't think this is a good idea — we shouldn't change the original logic. The original approach was better: throwing an exception for unknown types helps expose issues, which is better than just logging them. |
|
@goecho Thank you for your reply. I actually considered both options: throwing an exception to stop the client or just logging it. But since the issue is only logged in |
|
I think we can combine two ways. First, expand the ping type, and then use the error handler to support any errors |
|
Hi @ tzolov, ping is not an event, but a comment in the sse protocol that starts with ":". The official handling method is: it should be ignored. I think PR should be merged immediately because sending ping messages is present in many frameworks. exception |
|
@chengyi3192 thanks for clarifying the semantics of the comment SSE (non)events. |
|
@goecho I guess we can strengthen the check to verify that the |
- Replace McpError exceptions with debug/warning logs for unrecognized SSE event types - Continue processing instead of failing when encountering unknown SSE events - Update transport implementations: - WebClientStreamableHttpTransport: return empty tuple instead of throwing - WebFluxSseClientTransport: complete stream instead of erroring - HttpClientSseClientTransport: call sink.success() instead of sink.error() - HttpClientStreamableHttpTransport: return empty Flux for unknown events This improves client resilience when servers send non-standard or future SSE event types. Resolves modelcontextprotocol#272 , modelcontextprotocol#223 , modelcontextprotocol#93 Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
…ntLine() - Strengthen the logic for detecting SSE ping comments by checking that id, event, and data fields are empty. - Generalize comment detection into isCommentLine(ServerSentEvent) for clarity and reusability. - Improves consistency with the SSE specification and matches behavior of the Python SDK. - Replaces previous hardcoded check with a more robust and maintainable solution.
|
@tzolov Great point — I've now added the checks for id, event, and data, and pulled the logic into an isCommentLine() helper. Thanks! |
5 participants
Motivation and Context
How Has This Been Tested?
Breaking Changes
Types of changes
Checklist
Additional context